Add HashingUtils addToHash(Object[]) overloads - #12043
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 405bfd2 | Docs | Datadog PR Page | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 756cd02874
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
There was a problem hiding this comment.
The new Object[] overload changes the meaning of an existing call with a null literal: addToHash(0, null) now selects the array overload and throws NullPointerException instead of returning the unchanged seed. Array folding behavior is otherwise correct for mixed values, null elements, empty arrays, and bounded prefixes.
📊 Validated against 5 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 756cd02 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| return hash; | ||
| } | ||
|
|
||
| public static final int addToHash(int hash, Object[] arr) { |
There was a problem hiding this comment.
Null literal now throws through the array overload
Existing callers using a null literal can fail at runtime after recompiling against this version, instead of preserving the prior null-hashing behavior.
Assertion details
- Input: A caller invokes the pre-existing API as HashingUtils.addToHash(0, null).
- Expected:
The existing Object overload is selected and hashCode(null) contributes zero, returning the seed unchanged. - Actual:
The new Object[] overload is more specific for the null literal, so it calls arr.length and throws NullPointerException.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Mirror the array-folding helpers already present on LongHashingUtils: addToHash(hash, Object[]) and addToHash(hash, Object[], len), the latter folding only the first len elements. Split out of the client-side stats branch, where they were added but unused. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make the split-key contract explicit with javax.annotation: - addToHash(int/long, Object) and its null-folding delegate (hashCode/intHash) take @nullable — null hashes to a stable value. - addToHash(int/long, Object[], int) and the length-defaulting overload take @nonnull — a null array is a caller error, while individual elements may still be null (noted in Javadoc). This documents why a bare `addToHash(seed, null)` literal now binds to the Object[] overload rather than the single-Object one, and keeps the int and long hashing families symmetric. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
756cd02 to
405bfd2
Compare
What Does This Do?
Adds two array-folding helpers to
HashingUtils, mirroring overloads that already exist onLongHashingUtils:addToHash(int hash, Object[] arr)— folds every element onto the running hash.addToHash(int hash, Object[] arr, int len)— folds only the firstlenelements.It also annotates the nullness contract of the
addToHashoverloads (and their null-folding delegates) across bothHashingUtilsandLongHashingUtils: the single-Objectoverloads are@Nullable, and theObject[]overloads are@Nonnull(the array must be non-null; its elements may still be null).Motivation
Similar conveniences proved useful on
LongHashingUtilswhile working on client-side-stats, so they make sense for (Integer)HashingUtils, too. Restores parity withLongHashingUtils, which already carries the equivalent methods.The
@Nullable/@Nonnullpass documents why a bareaddToHash(seed, null)literal binds to theObject[]overload rather than the single-Objectone — turning a would-be silent overload-resolution surprise into a declared contract, kept symmetric across the int and long families (per Codex review feedback).Additional Notes
Unit tests cover the full-array fold (equivalence with
hash(Object[])), non-zero-seed carry-through, partial-lenfolding, andlen == 0(loop not entered), so both loop-branch outcomes are exercised.🤖 Generated with Claude Code